Skip to content

[memory/cryptolib] Launder status output#28810

Merged
nasahlpa merged 1 commit intolowRISC:masterfrom
siemen11:status_memory
Nov 26, 2025
Merged

[memory/cryptolib] Launder status output#28810
nasahlpa merged 1 commit intolowRISC:masterfrom
siemen11:status_memory

Conversation

@siemen11
Copy link
Contributor

Since the hardened memory operations just return OTCRYPTO_OK the compiler optimizes the status output which circumvents the control flow integrity. An example is

20011276: | | 9e2080e7 jalr
-1566(ra) # 2002bc54 <hardened_memcpy>

2001127a: | | 73900513 li
a0,1849

Launder the status output to circumvent this. Once there is another call in place which makes the possibility to return a different status, this launder is no longer needed.

The assembly files were checked for similar patterns, only a single call in RSA had a similar result.

Since the hardened memory operations just return OTCRYPTO_OK the
compiler optimizes the status output which circumvents the control flow
integrity. An example is

20011276:             |     |         9e2080e7                  jalr
		      -1566(ra) # 2002bc54 <hardened_memcpy>

2001127a:             |     |         73900513                  li
a0,1849

Launder the status output to circumvent this. Once there is another call
in place which makes the possibility to return a different status, this
launder is no longer needed.

The assembly files were checked for similar patterns, only a single call
in RSA had a similar result.

Signed-off-by: Siemen Dhooghe <[email protected]>
@siemen11 siemen11 requested a review from a team as a code owner November 23, 2025 21:05
@siemen11 siemen11 requested review from alees24, johannheyszl and nasahlpa and removed request for a team and alees24 November 23, 2025 21:05
@siemen11 siemen11 added the CherryPick:earlgrey_1.0.0 This PR should be cherry-picked to earlgrey_1.0.0 label Nov 23, 2025
Copy link
Contributor

@johannheyszl johannheyszl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @siemen11

Copy link
Member

@nasahlpa nasahlpa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Siemen!

A question - would it make sense to have this as a macro and replace OTCRYPTO_OK with this macro? Or is this a too heavy overhead and we only should use (status_t){.value = (int32_t)launder32((uint32_t)OTCRYPTO_OK.value)} where needed?

@siemen11
Copy link
Contributor Author

Thanks Siemen!

A question - would it make sense to have this as a macro and replace OTCRYPTO_OK with this macro? Or is this a too heavy overhead and we only should use (status_t){.value = (int32_t)launder32((uint32_t)OTCRYPTO_OK.value)} where needed?

Looks a lot of overhead to be honest. This should only be needed for "leafs" so functions that do not call other functions

Copy link
Member

@nasahlpa nasahlpa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Siemen, I agree - we only should use that pattern where needed.

@nasahlpa nasahlpa added this pull request to the merge queue Nov 26, 2025
Merged via the queue into lowRISC:master with commit 2b9f4df Nov 26, 2025
49 checks passed
@lowrisc-ci
Copy link

lowrisc-ci bot commented Nov 26, 2025

Successfully created backport PR for earlgrey_1.0.0:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CherryPick:earlgrey_1.0.0 This PR should be cherry-picked to earlgrey_1.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants